feat: Include /context hooks and conversation summary in /context show --expand
#1477
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1477 +/- ##
=======================================
Coverage 16.75% 16.75%
=======================================
Files 213 213
Lines 20704 20704
Branches 871 871
=======================================
Hits 3468 3468
Misses 17236 17236 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
evanliu048
changed the title
/context show/context show --expand
crates/chat-cli/src/cli/chat/mod.rs
Outdated
| } | ||
|
|
||
| // Prints hook configuration grouped by trigger: conversation sesiion start or per user message | ||
| fn print_hook_section(output: &mut impl Write, hooks: &HashMap<String, Hook>, trigger: HookTrigger) -> Result<()> { |
There was a problem hiding this comment.
This should take a &mut self receiver instead, otherwise let's place it as a standalone function rather than an associated function
Also for doc comments, use /// instead of //
There was a problem hiding this comment.
Thanks for the suggestion! 🙏
You're right — since the method doesn't use any fields from self, I've moved it out as a standalone function and updated the doc comment to use ///.
crates/chat-cli/src/cli/chat/mod.rs
Outdated
| style::Print("\n\n") | ||
| )?; | ||
| } else { | ||
| self.compact_history( |
There was a problem hiding this comment.
Definitely don't do this - just check if there is a summary cached, and if so print it.
There was a problem hiding this comment.
👌I’ve updated the code to check for a cached summary and print it if present, avoiding unnecessary regeneration.
crates/chat-cli/src/cli/chat/mod.rs
Outdated
| // Show last cached conversation summary if available, otherwise regenerate it | ||
| if expand { | ||
| if let Some(summary) = | ||
| self.conversation_state.latest_summary().map(|s| s.to_owned()) |
There was a problem hiding this comment.
This is inside the else block of if global_context_files.is_empty() && profile_context_files.is_empty() {, so it won't ever get executed if both global and profile context matches nothing.
Also shouldn't be necessary to to_owned here, we should be able to just print a reference
There was a problem hiding this comment.
Makes total sense. Moved the logic out of the else block and ditched the unnecessary .to_owned(). Learned something new again — thanks for the sharp eyes!
| execute!(self.output, style::Print("\n"))?; | ||
| } | ||
|
|
||
| // Show last cached conversation summary if available, otherwise regenerate it |
There was a problem hiding this comment.
nit - remove the "regenerate it" comment
There was a problem hiding this comment.
sure will remove this comment in future pr
3 participants
*Issue#, if available:*#1422
Description of changes:
This PR enhances the
/context showcommand by embedding associated hooks (both global and profile) under their respective sections and displaying a conversation summary for. Improves clarity and user experience.Screen.Recording.2025-05-05.at.1.56.39.PM.mov
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.